Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(KFLUXBUGS-1666): Only analyze build context for dependencies #1476

Merged
merged 1 commit into from
Sep 26, 2024
Merged

fix(KFLUXBUGS-1666): Only analyze build context for dependencies #1476

merged 1 commit into from
Sep 26, 2024

Conversation

arewm
Copy link
Member

@arewm arewm commented Sep 26, 2024

When generating the SBOM, we do not constrain the source analysis to the build context. This can result in many additional dependencies being included than should be.

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

@arewm arewm requested a review from a team September 26, 2024 19:12
@rcerven
Copy link
Contributor

rcerven commented Sep 26, 2024

/lgtm

lcarva
lcarva reviewed Sep 26, 2024
View reviewed changes
Copy link
Contributor

@lcarva lcarva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe? If the whole repo is available, doesn't that mean other parts could be used?

@arewm
Copy link
Member Author

arewm commented Sep 26, 2024

We run

unshare -Uf $UNSHARE_ARGS --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -w ${SOURCE_CODE_DIR}/$CONTEXT -- buildah build \
          $VOLUME_MOUNTS \
          "${BUILDAH_ARGS[@]}" \
          "${LABELS[@]}" \
          --tls-verify=$TLSVERIFY --no-cache \
          --ulimit nofile=4096:4096 \
          -f "$dockerfile_path" -t $IMAGE .

This sets the working directory to ${SOURCE_CODE_DIR}/$CONTEXT so no other context should be available.

https://man7.org/linux/man-pages/man1/unshare.1.html

@arewm
fix(KFLUXBUGS-1666): Only analyze build context for dependencies
e43916f 
When generating the SBOM, we do not constrain the source analysis to the
build context. This can result in many additional dependencies being
included than should be.

Signed-off-by: arewm <[email protected]>
@arewm
Copy link
Member Author

arewm commented Sep 26, 2024

The remaining Checkton issues are flagged for untouched code. Marking this PR to merge when ready.

@arewm arewm added this pull request to the merge queue Sep 26, 2024
Merged via the queue into konflux-ci:main with commit 0ea2f67 Sep 26, 2024
13 of 14 checks passed
@rh-tap-build-team rh-tap-build-team bot mentioned this pull request Sep 26, 2024
Merged
@arewm arewm deleted the kfluxbugs-1666 branch September 27, 2024 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rcerven rcerven rcerven approved these changes

@lcarva lcarva lcarva approved these changes

@chmeliik chmeliik Awaiting requested review from chmeliik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arewm @rcerven @lcarva